Fix to handle multiple # in vocab URIs. #1242
Conversation
|
Thanks for the PR @pulquero ! Can you explain what the problem is? As I understood it, you have URIs with more than one hash character (why?) and these break the current getId() method. Like @kinow said above, a test case would be very good to have so that we know what exactly is being fixed here. I wouldn't be surprised if such URIs caused problems elsewhere too - I don't recall encountering more than one hash character in URIs. |
|
It is uncommon but still valid. Pushed testcase. |
|
Kudos, SonarCloud Quality Gate passed!
|
There was a problem hiding this comment.
Thanks for the test @pulquero .
Not sure if this is a valid URL. Had a look at some SO with links to MDN and RFC's, but it looks like some browsers still try to handle the multiple hashes.
I opened a new Firefox tab, and typed about:config, then in the console window.location.hash was empty. Navigated to about:config#test, now the hash is #test, then about:config#test#prueba and now the has is #test#prueba.
In this test the hash/fragment appears to be multiHashVocabId. Can't say whether that's correct or not, but maybe somebody else can find another document or example to support one way or another 👍
|
I looked into it a bit deeper, and yes, multiple hashes are not valid. What I meant to do was #foo/bar, so I fixed that up on my side. With regards to this PR, I'm happy to withdraw it. But, maybe the existing behaviour should be changed as it neither throws an error that the input is invalid or gracefully does something that is unsuprising. Four options to be considered then: |
Thank you, it was interesting reading about it too (I never considered the possibility of multiple hashes.) Here's an interesting thread I found int he W3C list archives, though not conclusive 1
Sounds reasonable. Will leave it up to others, but I guess we could at least log a warn somewhere, if the decision is to not modify the code to handle the multiple hashes — no strong opinion from me. Thanks! Footnotes |
No description provided.